Add migration to reconcile stale plot_count data#260
Conversation
Idempotent UPDATE that sets storylines.plot_count and last_plot_time from COUNT(*) and MAX(block_timestamp) on the plots table. Only updates rows where values differ from computed aggregates. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
project7-interns
left a comment
There was a problem hiding this comment.
REQUEST CHANGES — one blocking issue.
Blocking: != doesn't match NULL values — stale rows with NULL last_plot_time will be skipped.
Line 16: s.plot_count != sub.cnt OR s.last_plot_time != sub.latest
In PostgreSQL, NULL != value evaluates to NULL (not TRUE), so any storyline where last_plot_time is currently NULL (never set) won't be updated by this migration — which is likely the most common stale case.
Fix: use IS DISTINCT FROM instead of !=:
WHERE s.storyline_id = sub.storyline_id
AND (s.plot_count IS DISTINCT FROM sub.cnt
OR s.last_plot_time IS DISTINCT FROM sub.latest);IS DISTINCT FROM treats NULL as a comparable value and returns TRUE when one side is NULL and the other isn't. Same applies to plot_count if it could be NULL.
Otherwise the SQL structure is correct — idempotent UPDATE with computed aggregates, only touches differing rows.
project7-interns
left a comment
There was a problem hiding this comment.
Verdict: REQUEST CHANGES
Summary
The migration is close, but the WHERE predicate uses SQL != comparisons that do not handle NULL correctly.
Findings
- [medium] Rows with
NULLinstorylines.last_plot_timecan be skipped even when they need repair. In PostgreSQL,NULL != sub.latestevaluates toNULL, notTRUE, so the wholeAND (...)condition may fail. Example: if a storyline already has the correctplot_countbut a staleNULLlast_plot_time, this migration will not update it.- File:
supabase/migrations/00013_reconcile_plot_counts.sql:16 - Suggestion: use
IS DISTINCT FROMfor both comparisons:s.plot_count IS DISTINCT FROM sub.cnt OR s.last_plot_time IS DISTINCT FROM sub.latest.
- File:
Decision
Request changes. The migration should use null-safe comparison so it reliably repairs all stale rows.
NULL != value evaluates to NULL in Postgres, silently skipping stale rows with NULL last_plot_time. IS DISTINCT FROM handles NULLs correctly. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
project7-interns
left a comment
There was a problem hiding this comment.
APPROVE
NULL-safety fix confirmed: IS DISTINCT FROM on both comparisons. Migration is now correct and idempotent — will properly catch stale rows regardless of NULL state.
project7-interns
left a comment
There was a problem hiding this comment.
Verdict: APPROVE
Summary
The follow-up patch fixes the migration predicate by using null-safe comparisons. The repair now correctly updates stale last_plot_time rows even when existing values are NULL.
Findings
- [info] No blocking issues found in the updated patch.
Decision
Approve. IS DISTINCT FROM makes the migration reliably idempotent and null-safe. CI was still pending when reviewed.
Adds $PLOT as a nav link pointing to /token page, consistent with existing nav link styling and responsive behavior. Fixes #260 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Summary
supabase/migrations/00013_reconcile_plot_counts.sqlto fix existing stalestorylines.plot_countandlast_plot_timevaluesCOUNT(*)andMAX(block_timestamp)fromplots— only updates rows where values differFixes #259
Test plan
npm run typecheckpasses🤖 Generated with Claude Code